fix(config): suppress workspace mount when workspaceMount is empty string#287
Conversation
…ring (#283) Per the devcontainer spec, setting workspaceMount to "" should suppress the default workspace mount. Previously the string type could not distinguish between omitted and explicitly empty, so "" fell through to the default bind mount. Change WorkspaceMount from string to *string so nil means omitted (use default mount) and *"" means suppress mount entirely.
📝 WalkthroughWalkthroughThis PR changes ChangesWorkspaceMount Pointer-Based Suppression
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for devsydev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/devcontainer/config.go (1)
199-204:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard against applying consistency to suppressed workspace mount.
When
WorkspaceMountis explicitly empty (suppressed), applying consistency creates an invalid mount string like,consistency='delegated'. ThemountSetConsistencyfunction appends consistency to the empty string, producing a malformed result that lacks type/source/target fields.🛡️ Proposed fix to skip consistency when mount is suppressed
+if options.WorkspaceMountConsistency != "" && substitutionContext.WorkspaceMount != "" { -if options.WorkspaceMountConsistency != "" { substitutionContext.WorkspaceMount = mountSetConsistency( substitutionContext.WorkspaceMount, options.WorkspaceMountConsistency, ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/devcontainer/config.go` around lines 199 - 204, The code applies mountSetConsistency to substitutionContext.WorkspaceMount even when the workspace mount was explicitly suppressed (empty), producing malformed mounts like ",consistency='delegated'"; fix by guarding the call so you only call mountSetConsistency(substitutionContext.WorkspaceMount, options.WorkspaceMountConsistency) when substitutionContext.WorkspaceMount is non-empty (and/or non-whitespace) and options.WorkspaceMountConsistency is set, leaving substitutionContext.WorkspaceMount untouched when it was intentionally suppressed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@pkg/devcontainer/config.go`:
- Around line 199-204: The code applies mountSetConsistency to
substitutionContext.WorkspaceMount even when the workspace mount was explicitly
suppressed (empty), producing malformed mounts like ",consistency='delegated'";
fix by guarding the call so you only call
mountSetConsistency(substitutionContext.WorkspaceMount,
options.WorkspaceMountConsistency) when substitutionContext.WorkspaceMount is
non-empty (and/or non-whitespace) and options.WorkspaceMountConsistency is set,
leaving substitutionContext.WorkspaceMount untouched when it was intentionally
suppressed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77d1876e-cc8e-4577-8827-3dd1906a136c
📒 Files selected for processing (9)
pkg/devcontainer/config.gopkg/devcontainer/config/config.gopkg/devcontainer/config/extends.gopkg/devcontainer/config/result.gopkg/devcontainer/config_test.gopkg/devcontainer/run.gopkg/devcontainer/run_test.gopkg/devcontainer/single.gopkg/driver/kubernetes/run.go
Summary
WorkspaceMountfromstringto*stringinNonComposeBaseconfig struct to distinguish between omitted (nil → default bind mount) and explicitly empty (""→ suppress mount), per the devcontainer specgetWorkspace()in run.go,substitute()in config.go, config merging in extends.go, mount construction in single.go, and mount collection in result.go to handle the three-way semantics: nil/default, empty/suppress, non-empty/use-as-isTestGetWorkspace_NilWorkspaceMountand updatesTestGetWorkspace_EmptyWorkspaceMountto verify suppression behaviorSummary by CodeRabbit